-
Notifications
You must be signed in to change notification settings - Fork 42
Update application shutdown logic #1760
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
31b61f2 to
8da103c
Compare
...loudfoundry/multiapps/controller/core/application/shutdown/ApplicationShutdownScheduler.java
Outdated
Show resolved
Hide resolved
...iapps-controller-core/src/main/java/org/cloudfoundry/multiapps/controller/core/Messages.java
Outdated
Show resolved
Hide resolved
...loudfoundry/multiapps/controller/core/application/shutdown/ApplicationShutdownScheduler.java
Outdated
Show resolved
Hide resolved
...src/main/java/org/cloudfoundry/multiapps/controller/persistence/dto/ApplicationShutdown.java
Outdated
Show resolved
Hide resolved
.../main/java/org/cloudfoundry/multiapps/controller/persistence/dto/ApplicationShutdownDto.java
Outdated
Show resolved
Hide resolved
...t/src/main/java/org/cloudfoundry/multiapps/controller/shutdown/client/util/ShutdownUtil.java
Outdated
Show resolved
Hide resolved
...t/src/main/java/org/cloudfoundry/multiapps/controller/shutdown/client/util/ShutdownUtil.java
Outdated
Show resolved
Hide resolved
| instanceShutdownExecutor.execute(applicationGuid, i); | ||
| public void execute(String applicationId, int applicationInstanceCount) { | ||
| ApplicationShutdownScheduler applicationShutdownScheduler = getApplicationShutdownScheduler(); | ||
| List<ApplicationShutdown> scheduledApplicationShutdowns = applicationShutdownScheduler.scheduleApplicationForShutdown(applicationId, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What will happen if count of instances is mistaken or some of existing instances are in crashed/not running state? Will it lead to infinite loop?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i dont it will because if an instance has crashed on startup, the instance will delete the scheduled entry from the database so there won't be attempts for shutdown.
also if we scheduled 5 instances for shutdown but in reality there are 3, we will shutdown the intances from 1-3. the entries for shutdown for instance 4 and 5 won't be cleared from the database. when we increase the instances back to 5, the scheduled entries will be cleared from the database.
If we schedule 3 instances but there are 5, only the first 3 will be shutdown. the other 2 won't be shutdown
...in/java/org/cloudfoundry/multiapps/controller/web/resources/ApplicationShutdownResource.java
Show resolved
Hide resolved
...-web/src/main/java/org/cloudfoundry/multiapps/controller/web/bootstrap/BootstrapServlet.java
Outdated
Show resolved
Hide resolved
...loudfoundry/multiapps/controller/core/application/shutdown/ApplicationShutdownScheduler.java
Outdated
Show resolved
Hide resolved
...a/org/cloudfoundry/multiapps/controller/persistence/services/ApplicationShutdownService.java
Outdated
Show resolved
Hide resolved
.../main/java/org/cloudfoundry/multiapps/controller/persistence/dto/ApplicationShutdownDto.java
Show resolved
Hide resolved
...g/cloudfoundry/multiapps/controller/persistence/query/impl/ApplicationShutdownQueryImpl.java
Outdated
Show resolved
Hide resolved
...src/main/java/org/cloudfoundry/multiapps/controller/process/jobs/ApplicationShutdownJob.java
Outdated
Show resolved
Hide resolved
| private void shutdownApplication(ApplicationShutdown applicationShutdown) { | ||
| CompletableFuture.runAsync(() -> { | ||
| logProgressOfShuttingDown(applicationShutdown, Messages.SHUTTING_DOWN_APPLICATION_WITH_ID_AND_INDEX); | ||
| flowableFacade.shutdownJobExecutor(); | ||
| }) | ||
| .thenRun(() -> logProgressOfShuttingDown(applicationShutdown, Messages.SHUT_DOWN_APPLICATION_WITH_ID_AND_INDEX)); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible here, exception to be thrown inside the async task, this will impact the DB state in a wrong way. Maybe add FAILED status for such cases.
|
Also add some unit tests, cause now there is 0.0% Coverage on New Code |
4f3dab1 to
aea5e01
Compare
LMCROSSITXSADEPLOY-3344 # Conflicts: # multiapps-controller-process/src/main/java/org/cloudfoundry/multiapps/controller/process/Messages.java # Conflicts: # multiapps-controller-core/src/main/java/org/cloudfoundry/multiapps/controller/core/Messages.java # multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/resources/ApplicationShutdownResource.java
aea5e01 to
1879759
Compare
1879759 to
22db5eb
Compare
|



No description provided.